[ECO-5009][CHA-RL1] Room ATTACH #27

sacOO7 · 2024-10-03T12:30:21Z

Fixed #21

coderabbitai · 2024-10-03T12:30:28Z

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

messages, typing and reactions channel attach fails

extends internal eventemitter

…ssing impl. for EventEmitter apply

RoomLifecYCLE management

# Conflicts: # chat-android/src/main/java/com/ably/chat/RoomOptions.kt # chat-android/src/main/java/com/ably/chat/RoomStatus.kt

…erred

…ributor across all contributors and initialized RoomLifeCycle inside Room

…nels

attach method to call lifecyclemanager attach method instead

…ssing attach suspending method

ttypic · 2024-11-05T09:52:28Z

chat-android/src/main/java/com/ably/chat/ErrorCodes.kt

@@ -3,105 +3,106 @@ package com.ably.chat
 /**
 * Error codes for the Chat SDK.
 */
-object ErrorCodes {


Why do you think enum here is better?

Having type for given ErrorCode enforces compile time check for right assertions with reusability.
Also, IDE gives proper suggestions when matching error codes, unlike when it's generic Int.
This makes it easy to write ErrorCode assertions in code as well as tests.

Also, in chat-js, ErrorCode is represented as an enum, this makes it super easy to correlate between two codebases.

Not sure I understand, could you give an example? Typescript's enums are nothing like Kotlin's enums. Error codes usually come from backend, and they come as Int. I currently don't understand what benefits we get, except we have to additionally serialize/deserialize Int into enum

Sure, I am closing this PR in favor of #33. We can continue discussion for the same there 👍

ttypic · 2024-11-05T09:54:51Z

chat-android/src/main/java/com/ably/chat/RoomStatus.kt

@@ -118,3 +162,60 @@ data class RoomStatusChange(
     */
    val error: ErrorInfo? = null,
 )
+
+open class RoomStatusEvenEmitter : EventEmitter<RoomLifecycle, RoomStatus.Listener>() {


here is typo in the name, should be ...EventEmitter

Fixed ce9c0ba

ttypic · 2024-11-05T09:56:54Z

chat-android/src/main/java/com/ably/chat/RoomStatus.kt

+    }
+}
+
+class DefaultStatus(private val logger: LogHandler?) : InternalRoomStatus, RoomStatusEvenEmitter() {


It's not Kotlin idiomatic to use inheritance here. It's much better to use composition

Fixed ce9c0ba

Refactored DefaultStatus, using composition over inheritance for RoomStatusEventEmitter

# Conflicts: # chat-android/src/main/java/com/ably/chat/RoomReactions.kt

sacOO7 · 2024-11-08T11:49:26Z

Superseded by #33

Added checks for current room state before room ATTACH operation

6d2d24c

sacOO7 changed the title ~~Added checks for current room state before room ATTACH operation~~ Feature - Room ATTACH Oct 3, 2024

sacOO7 changed the title ~~Feature - Room ATTACH~~ [ECO-5009][CHA-RL1]Feature - Room ATTACH Oct 3, 2024

sacOO7 changed the title ~~[ECO-5009][CHA-RL1]Feature - Room ATTACH~~ [ECO-5009][CHA-RL1 Room ATTACH Oct 3, 2024

sacOO7 changed the title ~~[ECO-5009][CHA-RL1 Room ATTACH~~ [ECO-5009][CHA-RL1] Room ATTACH Oct 3, 2024

sacOO7 added 4 commits October 8, 2024 17:46

Implemented RoomStatus interface, added code to handle exceptions when

d018b0d

messages, typing and reactions channel attach fails

Marked RoomOptions as optional as per spec

99c7363

Added offAll method interface method to remove all listeners

3067774

Refactored room interfaces, implemented DefaultRoomStatus class that

8ffabb1

extends internal eventemitter

sacOO7 force-pushed the feature/room-ATTACH branch from 34add29 to 8ffabb1 Compare October 11, 2024 14:01

sacOO7 added 16 commits October 14, 2024 17:53

Replaced ChatEventEmitter with RoomStatusEventEmitter, implemented mi…

5b321c6

…ssing impl. for EventEmitter apply

Added proper logging for DefaultRoom room attach operation failure

7943aba

Added RoomLifecycleManager class, defined basic interfaces needed for

21ac68a

RoomLifecYCLE management

Updated coroutine core as a direct dependency to chat-android package

0d7f9a9

Implemented missing interfaces as a part of RoomLifeCycleManager

ba0f81e

Merge branch 'main' into feature/room-ATTACH

5daeaf3

# Conflicts: # chat-android/src/main/java/com/ably/chat/RoomOptions.kt # chat-android/src/main/java/com/ably/chat/RoomStatus.kt

Fixed linting issues recommended by detekt

899363b

Updated ably-java dependency to latest version

102513c

Updated ContributesToRoomLifecycle channel property to CompletableDef…

572fd8d

…erred

Upgraded kotlinx.coroutines dependency to latest version

da00860

Renamed DefaultRoomStatusStatus class to DefaultStatus, same as chat-js

be11428

Implemented basic definition for RoomLifeCycleManager

a68a2a7

Added interface impl. for ContributesToRoomLifecycle and ResolvedCont…

a5e837c

…ributor across all contributors and initialized RoomLifeCycle inside Room

Fixed linting errors in local files

5dfb474

Made AblyRealtimeChannel as a explicit type for all room feature chan…

e0d5dab

…nels

Added TODOs before initializing RoomLifeCycleManager, refactored room

0460c5c

attach method to call lifecyclemanager attach method instead

sacOO7 force-pushed the feature/room-ATTACH branch from 1f5623f to 2c3655a Compare October 22, 2024 10:32

Removed unused coroutinescope from the RoomLifeCycleManager, added mi…

b5a5e54

…ssing attach suspending method

sacOO7 force-pushed the feature/room-ATTACH branch from 2c3655a to b5a5e54 Compare October 22, 2024 10:40

This was referenced Oct 23, 2024

[CHA-RL1] Implement Room ATTACH operation #21

Open

[ECO-5009][CHA-RL1] Room ATTACH with retry #33

Open

ttypic reviewed Nov 5, 2024

View reviewed changes

sacOO7 added 2 commits November 8, 2024 15:35

Fixed RoomStatusEventEmitter typo,

ce9c0ba

Refactored DefaultStatus, using composition over inheritance for RoomStatusEventEmitter

Merge branch 'main' into feature/room-ATTACH

d689419

# Conflicts: # chat-android/src/main/java/com/ably/chat/RoomReactions.kt

sacOO7 force-pushed the feature/room-ATTACH branch from b8c3cc7 to d689419 Compare November 8, 2024 10:16

sacOO7 closed this Nov 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ECO-5009][CHA-RL1] Room ATTACH #27

[ECO-5009][CHA-RL1] Room ATTACH #27

sacOO7 commented Oct 3, 2024 •

edited

Loading

coderabbitai bot commented Oct 3, 2024 •

edited

Loading

Review skipped

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

ttypic Nov 5, 2024

sacOO7 Nov 8, 2024 •

edited

Loading

sacOO7 Nov 8, 2024

ttypic Nov 8, 2024

sacOO7 Nov 8, 2024

ttypic Nov 5, 2024

sacOO7 Nov 8, 2024

ttypic Nov 5, 2024

sacOO7 Nov 8, 2024

sacOO7 commented Nov 8, 2024

[ECO-5009][CHA-RL1] Room ATTACH #27

[ECO-5009][CHA-RL1] Room ATTACH #27

Conversation

sacOO7 commented Oct 3, 2024 • edited Loading

coderabbitai bot commented Oct 3, 2024 • edited Loading

Review skipped

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Choose a reason for hiding this comment

sacOO7 Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sacOO7 commented Nov 8, 2024

sacOO7 commented Oct 3, 2024 •

edited

Loading

coderabbitai bot commented Oct 3, 2024 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

sacOO7 Nov 8, 2024 •

edited

Loading